Skip to content

fix: guard None refusal in ItemHelpers.extract_last_content#3427

Closed
ioleksiuk wants to merge 1 commit into
openai:mainfrom
ioleksiuk:fix/guard-none-refusal-extract-last-content
Closed

fix: guard None refusal in ItemHelpers.extract_last_content#3427
ioleksiuk wants to merge 1 commit into
openai:mainfrom
ioleksiuk:fix/guard-none-refusal-extract-last-content

Conversation

@ioleksiuk
Copy link
Copy Markdown
Contributor

Summary

Follow-up to #3394 (just merged) that fixed the text branch of `extract_last_content` against `None` from `model_construct` / provider gateway paths.

The refusal branch at `src/agents/items.py:693-694` has the same `-> str` return type contract and the same Pydantic-typed `str` source field (`ResponseOutputRefusal.refusal`), so it can violate the contract in the same way:

```python

from openai.types.responses import ResponseOutputMessage, ResponseOutputRefusal
from agents.items import ItemHelpers
refusal_part = ResponseOutputRefusal.model_construct(refusal=None, type="refusal")
msg = ResponseOutputMessage.model_construct(
... id="m", role="assistant", status="completed",
... type="message", content=[refusal_part])
ItemHelpers.extract_last_content(msg)
None # but the function is typed -> str
```

Apply the same `or ""` coercion and rationale comment that the text branch now has (items.py:687-691).

Scope (apologies for the noise on #3394)

Same scope as the merged version of #3394 — only the `-> str` branches get the coercion. `extract_last_text` (declared `-> str | None`) is intentionally left alone because:

  • `None` is already a legal return per its signature.
  • Coalescing `""` → `None` would silently change semantics for callers that distinguish empty text from missing text.

This PR is the equivalent fix on the refusal branch that I missed in #3394's first scope.

Test plan

  • New `test_extract_last_content_returns_empty_string_for_none_refusal` — verifies the type contract holds for `refusal=None`.
  • New `test_extract_last_content_returns_refusal_normally` — verifies regular refusal text still passes through unchanged.
  • `uv run pytest tests/utils/test_pretty_print_and_items.py tests/test_items_helpers.py` — 43 passed.
  • `uv run ruff check` on the touched files — All checks passed.

Issue number

N/A — direct follow-up to merged #3394.

Checks

  • I've added new tests (if relevant)
  • I've added/updated the relevant documentation (no doc changes needed)
  • I've run `make lint` and `make format`
  • I've made sure tests pass

Follow-up to openai#3394 (just merged) that fixed the text branch of
`extract_last_content` against `None` from `model_construct` / provider
gateway paths. The refusal branch has the same `-> str` return type
contract and the same Pydantic-typed `str` source field, so it can
violate the contract in the same way:

    >>> from openai.types.responses import (
    ...     ResponseOutputMessage, ResponseOutputRefusal,
    ... )
    >>> from agents.items import ItemHelpers
    >>> refusal_part = ResponseOutputRefusal.model_construct(
    ...     refusal=None, type="refusal")
    >>> msg = ResponseOutputMessage.model_construct(
    ...     id="m", role="assistant", status="completed",
    ...     type="message", content=[refusal_part])
    >>> ItemHelpers.extract_last_content(msg)
    None        # but the function is typed `-> str`

Apply the same `or ""` coercion and rationale comment that the text
branch now has (items.py:687-691).

Note: `extract_last_text` (declared `-> str | None`) is intentionally
left alone — same reasoning as in openai#3394's final scope.
@seratch
Copy link
Copy Markdown
Member

seratch commented May 16, 2026

@ioleksiuk Can you share the actual examples of this issue? I am unsure if this is a real issue.

@ioleksiuk
Copy link
Copy Markdown
Contributor Author

@seratch Thank you for reviewing the PR!
After a closer look, I feel like it's okay to close it

@ioleksiuk ioleksiuk closed this May 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants